-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Allow TypeVarTuple arguments to subclasses of generic TypedDict #19234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Allow TypeVarTuple arguments to subclasses of generic TypedDict #19234
Conversation
This comment has been minimized.
This comment has been minimized.
|
Apologies, I removed the |
This comment has been minimized.
This comment has been minimized.
Yes, every test that depends on python version higher than minimal supported version (3.9 now) should live in |
This comment has been minimized.
This comment has been minimized.
|
@sterliakov Thanks for taking a look, and for the suggestion. I've moved the tests using |
|
@sterliakov Sorry to ping you again, but perhaps you could review this PR or maybe suggest a member who could? For what it's worth, the code change is pretty minimal and the test coverage should be sufficient, IMHO. Thanks in advance! |
sterliakov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG, thank you! The test suite looks solid.
You don't have to deal with that here, so just a quick question if you have time: do equivalent tests pass with NamedTuple? They have a lot in common, and many problems affecting TypedDict also affect NamedTuple of similar structure.
Let's see if @hauntsaninja has time to review and merge this, I don't have write perms here:)
|
Thanks @sterliakov! About the class MyDict(TypedDict):
foo: int
class MySubDict(MyDict):
bar: int
msd = MySubDict(foo=1, bar=2) # Fine
class MyTuple(NamedTuple):
foo: int
class MySubTuple(MyTuple):
bar: int
mst = MySubTuple(foo=1, bar=2) # Error: unexpected keyword "bar"I sometimes think it would be nice to be able to extend |
2632de8 to
94ed01f
Compare
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
|
@hauntsaninja I understand that (apparently) this isn't an issue that many folks run into, but I think you'll agree that it actually is a bug that may as well be fixed. I've rebased the branch to latest master just now. |
Fixes #16975
Supersedes #16977
Allows
TypeVarTuples as type argument for subclasses of genericTypedDicts.Since (1) TypedDict became generic and (2) TypeVarTuple was introduced, mypy now supports creating generic typed dictionaries with variadic type arguments. However, things went wrong when deriving a subclass from such a generic typeddict.
Full disclosure: I submitted an earlier PR for this issue, but it's been a while. Earlier I wasn't 100% sure about one minor aspect of the changes I made, but in the mean time the context has changed to the point that that aspect is no longer relevant.
For tests, I started with a copy of an existing one from test-data/unit/check-typevar-tuple.test, and added two subclasses: one which just passes a TypeVarTuple generic type parameter up to the base class, and another which passes a TypeVarTuple and adds a regular TypeVar argument.
I did this all of this twice: once using the
Unpack[Ts]syntax and once using the*Tssyntax.